Add optional background thread for JWKS refresh #192

Copilot · 2025-12-09T18:17:40Z

Background Thread for JWKS Refresh - Latest Updates

Recent Changes (This Commit)

Linter fixes

Fixed clang-format violations in src/scitokens_monitoring.cpp
Adjusted line wrapping for long function calls to comply with formatting rules
All modified files now pass clang-format --dry-run --Werror check

Previous Changes

Statistics and Monitoring

Added comprehensive monitoring statistics for JWKS operations
Tracks successful/failed validations, key lookups, and background refreshes
JSON output for easy integration with monitoring systems
Per-issuer statistics with timing information

1. Include expired entries for refresh

get_all_issuers_from_db() now returns expired entries instead of skipping them
Expired entries have next_update set to now for immediate refresh
Handles cases where system was down for extended period

2. Optimized atomic operations

Changed all atomic operations from default sequential consistency to acquire/release semantics
Applied to m_shutdown and m_running atomics in BackgroundRefreshManager
Provides necessary synchronization with better performance
Load operations use std::memory_order_acquire
Store operations use std::memory_order_release

3. Thread Management

BackgroundRefreshManager singleton that supports multiple start/stop cycles
Graceful shutdown via condition variable, not polling
Automatic start on first verification if enabled (via std::once_flag in Validator)
Database-driven issuer discovery instead of explicit tracking

Configuration

keycache.refresh_interval_ms - wake interval N (default: 60s)
keycache.refresh_threshold_ms - refresh threshold M (default: 10min)
Default: disabled for backward compatibility

Testing

All integration tests pass
BackgroundRefreshTest validates thread lifecycle
Database-driven refresh works correctly including expired entries
Monitoring statistics tests validate JSON output

Original prompt

This section details on the original issue you should resolve

<issue_title>Optional background thread for JWKS refresh</issue_title>
<issue_description>It stinks to pause a verification for a refresh, especially if the JWKS server took that exact second to respond slowly.

The library should have an optional background thread (enabled / disabled by invoking an appropriately-named keycache_* function)

This should default to off but, based on the library config at load time (in issue #189 we will provide a new way to manipulate config) it can be turned on. If enabled at load time, the thread launches at load time. If enabled after load - but before the first verify (using a once flag) - it will launch on first verify. A condition variable should be used to allow the thread to gracefully wake and shutdown when it isn't refreshing. At library unload (or when the public API to stop the background thread is invoked), the thread should be shutdown.

The main loop of the background thread should be to wake up every N milliseconds, iterate through every known issuer and, if the next update is within M milliseconds, try to refresh the JWKS. N and M should be configurable, defaulting to 60000 (60 seconds) and 600000 (10 minutes).

An integration test should be provided that reduces the values N and M so the test observes the refreshes.

In future work, we will keep monitoring statistics and count the number of successful and failed background refreshes.</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes Optional background thread for JWKS refresh #191

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

bbockelm

@copilot - please address code review comments and the linter issues.

src/scitokens_internal.cpp

src/scitokens_internal.h

src/scitokens_internal.cpp

Copilot · 2025-12-09T19:19:09Z

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

demo.scitokens.org
- Triggering command: /home/REDACTED/work/scitokens-cpp/scitokens-cpp/build/test/scitokens-gtest /home/REDACTED/work/scitokens-cpp/scitokens-cpp/build/test/scitokens-gtest /tmp/ccWTvQxO.s t_access.cpp.o u/13/cc1plus /var�� k/scitokens-cpp/scitokens-cpp/ve/tmp/ccNXYCxO.s scitokens-cpp/vendor/jwt-cpp/include ate_jwks.cpp.o scitokens-cpp/srbash (dns block)
- Triggering command: /home/REDACTED/work/scitokens-cpp/scitokens-cpp/build/test/scitokens-gtest /home/REDACTED/work/scitokens-cpp/scitokens-cpp/build/test/scitokens-gtest --color= (dns block)
esm.ubuntu.com
- Triggering command: /usr/lib/apt/methods/https /usr/lib/apt/methods/https (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

bbockelm

@copilot - please address code review items and linter.

src/scitokens_cache.cpp

src/scitokens_internal.cpp

Copilot · 2025-12-09T20:24:02Z

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

esm.ubuntu.com
- Triggering command: /usr/lib/apt/methods/https /usr/lib/apt/methods/https (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

bbockelm · 2025-12-09T20:40:40Z

@copilot - address the linter issues.

Copilot · 2025-12-09T20:45:18Z

@copilot - address the linter issues.

Fixed in commit 6f3a1de. Applied clang-format to all modified files. The integration test had line wrapping and indentation issues that are now corrected. All files pass clang-format --dry-run --Werror check.

bbockelm · 2025-12-10T14:14:35Z

@copilot - address linter issues. Run clang-format and ensure there are no problems.

Copilot · 2025-12-10T14:19:46Z

@copilot - address linter issues. Run clang-format and ensure there are no problems.

All files already pass clang-format checks. Verified:

All 6 modified C++ files pass clang-format --dry-run --Werror with no issues
No formatting changes needed (diff check confirms files are already formatted)
Build completes successfully with no warnings or errors
All integration tests pass (100%)

The linter issues were previously fixed in commit 6f3a1de. No additional changes required.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

esm.ubuntu.com
- Triggering command: /usr/lib/apt/methods/https /usr/lib/apt/methods/https (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>

…atabase to get issuers - Remove once_flag from BackgroundRefreshManager, allowing multiple start/stop cycles - Move once_flag to Validator class for first-time initialization on verification - Remove issuer tracking (add_issuer, get_issuers, m_issuers map) - Add get_all_issuers_from_db() method to retrieve issuers from SQLite cache - Update refresh_loop to query database instead of tracking issuers - Improve thread safety in start/stop methods with proper mutex handling - Format code with clang-format Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>

…antics - Include expired entries in get_all_issuers_from_db() for refresh after long downtime - Set next_update to now for expired entries so they get refreshed immediately - Use std::memory_order_acquire/release for all atomic operations instead of sequential consistency - Improves performance while maintaining proper synchronization guarantees Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>

- Apply clang-format to test/integration_test.cpp - Fix line wrapping and indentation to comply with LLVM style - No functional changes, only formatting adjustments Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>

- Add background_successful_refreshes and background_failed_refreshes counters to IssuerStats for tracking per-issuer background refresh results - Add is_running() method to BackgroundRefreshManager to check thread state - Track statistics in refresh_loop() when JWKS refresh succeeds or fails - Add maybe_write_monitoring_file_from_verify() that skips file writes when background refresh thread is running (to avoid redundant writes) - Write monitoring file from background thread at end of each refresh cycle - Update get_json() to include new background refresh statistics - Update integration test to verify background refresh via monitoring API using keycache_set_jwks() to force cache entry with short update interval

- Change MonitoringStats::get_json() to cast integer counters to int64_t instead of double for proper integer representation in JSON - Update monitoring_test.cpp to parse integer fields using is<int64_t>() and get<int64_t>() instead of double - Use memory_order_relaxed for atomic loads in get_json() since these are read-only statistics snapshots

bbockelm · 2025-12-10T19:29:14Z

@copilot - please fix remaining clang-format issues found by the linter.

- Apply clang-format to fix line wrapping in monitoring statistics code - Adjust formatting for long picojson::value() calls - All files now pass clang-format --Werror check Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>

Copilot · 2025-12-10T19:34:02Z

@copilot - please fix remaining clang-format issues found by the linter.

Fixed in commit 1b2c830. Applied clang-format to src/scitokens_monitoring.cpp which had line wrapping violations. All 10 modified files now pass clang-format --dry-run --Werror with no issues.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

this-hostname-does-not-exist-12345.invalid
- Triggering command: /home/REDACTED/work/scitokens-cpp/scitokens-cpp/build/test/scitokens-integration-test /home/REDACTED/work/scitokens-cpp/scitokens-cpp/build/test/scitokens-integration-test (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Copilot AI assigned Copilot and bbockelm Dec 9, 2025

Copilot started work on behalf of bbockelm December 9, 2025 18:18 View session

Copilot AI changed the title ~~[WIP] Add optional background thread for JWKS refresh~~ Add optional background thread for JWKS refresh Dec 9, 2025

Copilot AI requested a review from bbockelm December 9, 2025 18:39

Copilot finished work on behalf of bbockelm December 9, 2025 18:39

bbockelm requested changes Dec 9, 2025

View reviewed changes

src/scitokens_internal.cpp Outdated Show resolved Hide resolved

src/scitokens_internal.cpp Outdated Show resolved Hide resolved

src/scitokens_internal.h Outdated Show resolved Hide resolved

src/scitokens_internal.cpp Outdated Show resolved Hide resolved

Copilot started work on behalf of bbockelm December 9, 2025 19:01 View session

Copilot AI requested a review from bbockelm December 9, 2025 19:19

Copilot finished work on behalf of bbockelm December 9, 2025 19:19

bbockelm requested changes Dec 9, 2025

View reviewed changes

src/scitokens_cache.cpp Show resolved Hide resolved

src/scitokens_internal.cpp Outdated Show resolved Hide resolved

Copilot started work on behalf of bbockelm December 9, 2025 20:17 View session

Copilot AI requested a review from bbockelm December 9, 2025 20:24

Copilot finished work on behalf of bbockelm December 9, 2025 20:24

Copilot started work on behalf of bbockelm December 9, 2025 20:41 View session

Copilot finished work on behalf of bbockelm December 9, 2025 20:47

Copilot started work on behalf of bbockelm December 10, 2025 14:15 View session

Copilot finished work on behalf of bbockelm December 10, 2025 14:21

Copilot AI and others added 6 commits December 10, 2025 14:28

Initial plan

3309c28

Implement background JWKS refresh infrastructure

d90cb2a

Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>

Add integration test for background JWKS refresh

cfc2d16

Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>

Address code review feedback - improve thread safety and documentation

d796131

Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>

Copilot AI and others added 2 commits December 10, 2025 18:26

Fix clang-format linter issues in integration test

5b0b862

- Apply clang-format to test/integration_test.cpp - Fix line wrapping and indentation to comply with LLVM style - No functional changes, only formatting adjustments Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>

bbockelm force-pushed the copilot/add-background-thread-jwks-refresh branch from 6f3a1de to 0c973ff Compare December 10, 2025 18:45

bbockelm added 2 commits December 10, 2025 19:12

Merge remote-tracking branch 'origin/master' into pr-192

79bb536

Copilot started work on behalf of bbockelm December 10, 2025 19:29 View session

Copilot finished work on behalf of bbockelm December 10, 2025 19:36

bbockelm marked this pull request as ready for review December 10, 2025 19:38

bbockelm merged commit a569ab5 into master Dec 10, 2025
21 checks passed

Add optional background thread for JWKS refresh #192

Add optional background thread for JWKS refresh #192

Uh oh!

Conversation

Copilot AI commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background Thread for JWKS Refresh - Latest Updates

Recent Changes (This Commit)

Previous Changes

Comments on the Issue (you are @copilot in this section)

Uh oh!

bbockelm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Dec 9, 2025

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

bbockelm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Dec 9, 2025

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

bbockelm commented Dec 9, 2025

Uh oh!

Copilot AI commented Dec 9, 2025

Uh oh!

bbockelm commented Dec 10, 2025

Uh oh!

Copilot AI commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

bbockelm commented Dec 10, 2025

Uh oh!

Copilot AI commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Dec 9, 2025 •

edited

Loading

Copilot AI commented Dec 10, 2025 •

edited

Loading

Copilot AI commented Dec 10, 2025 •

edited

Loading